feat(sandbox): persistent per-user sandbox container (V1 Phase A) - #6584

Derive the sandbox workspace from the invoking user

For HostedSingleTenantVolumeSandboxed, this computes the abstract /workspace mount once from the boot owner and mounts that single directory into the shared root filesystem. In a hosted single-tenant deployment with multiple authenticated users, builtin.read_file/builtin.write_file for every non-owner user will resolve /workspace to the owner's sandbox directory, while that user's shell container still derives its bind from request.scope.user_id; this both leaks the owner workspace and breaks shell/filesystem parity for other users.

AGENTS.md reference: crates/AGENTS.md:L207-L210

Useful? React with 👍 / 👎.

Sanitize sandbox command output before returning it

When the tenant-sandbox path is used, Docker stdout/stderr are returned directly from collect_exec_output; unlike the host process path (capture_command_output), this never runs the command-output LeakDetector that blocks or redacts secret-looking output. A sandboxed shell command that prints a .env value or API key-shaped token can therefore send it back to the model verbatim.

AGENTS.md reference: crates/ironclaw_host_runtime/AGENTS.md:L27-L28

Useful? React with 👍 / 👎.

Apply sandbox concurrency limits to every user

This sets the sandbox SpawnProcess ceiling only for the boot owner. The ReserveResources obligation later reserves against the actual invocation's ResourceScope user, so any other authenticated user in the hosted single-tenant sandboxed profile has no configured ceiling and can launch containers without the intended per-user cap.

AGENTS.md reference: crates/AGENTS.md:L207-L209

Useful? React with 👍 / 👎.

Check the worker image before skipping staging builds

The skipped summary now claims both ironclaw:staging and ironclaw-worker:staging are current, but the preceding skip check only pulls/inspects IMAGE_NAME:staging. If a prior run pushed the main image and failed before pushing or smoking the worker image, the next scheduled/staging run will set skip=true, skip the worker build and smoke job, and leave the worker tag missing or stale.

Useful? React with 👍 / 👎.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Verify the worker image before skipping.

The skip gate only pulls and inspects ${IMAGE_NAME}:staging. If ironclaw-worker:staging is missing or stale, this run skips its build and the worker smoke job while claiming both images are current. Inspect the worker image’s ironclaw.git.sha too, and skip only when both labels match SOURCE_SHA.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/docker.yml around lines 260 - 262, Update the
staging-image skip gate in the workflow to pull and inspect both
IMAGE_NAME:staging and IMAGE_NAME_WORKER:staging, including each image’s
ironclaw.git.sha label. Only skip the builds and worker smoke job when both
labels match SOURCE_SHA; otherwise continue the build path.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Align the lower-bound clamping contract. output_limit: 0 is rejected, but the manifest and parser docs promise values outside the range are clamped and only malformed values are rejected.

• crates/ironclaw_host_runtime/src/first_party_tools/shell.rs#L41-L43: limit the claim to valid positive numeric values, or accept zero and clamp it.
• crates/ironclaw_host_runtime/src/first_party_tools/shell_core.rs#L151-L167: correct “under-floor values are clamped” and “only rejects malformed value” to match the zero-value rejection.

As per coding guidelines, “Comments promising cross-layer guarantees must be enforced by code or tests, or softened to describe intent.”

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_host_runtime/src/first_party_tools/shell.rs` around lines 41
- 43, The documented lower-bound behavior for output_limit conflicts with its
zero-value rejection. In
crates/ironclaw_host_runtime/src/first_party_tools/shell.rs lines 41-43, either
revise the description to cover only valid positive numeric values or update the
implementation to accept and clamp zero; in
crates/ironclaw_host_runtime/src/first_party_tools/shell_core.rs lines 151-167,
align the parsing comments with the chosen behavior by removing claims that
under-floor values are clamped and that only malformed values are rejected when
zero remains invalid.

Source: Coding guidelines

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reserve space for the truncation marker.

The function retains limit bytes of head/tail plus the marker, so it exceeds the requested cap. For the new test’s 4,106-byte input and 4,096-byte limit, the result is larger than the input, making Line 640 fail. Budget head/tail from limit - marker.len() and assert truncated.len() <= limit.

As per path instructions, “Apply explicit limits to user-controlled … process output.”

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_host_runtime/src/process_output.rs` around lines 576 - 587,
The truncate_output_to function currently budgets limit bytes for content
without reserving space for the truncation marker, allowing the result to exceed
the requested cap. Calculate the marker length first, budget the head and tail
from limit minus that length, and ensure the returned truncated output never
exceeds limit, including for the 4,096-byte case.

Source: Path instructions

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Honor output_limit on the host-process path.

The shared shell schema advertises this as a per-call captured-output bound, but this branch explicitly ignores it; a local-profile request for 1 KiB can still capture/save the fixed-size output. Enforce the same bound in execute_local_command/process_output, and cover it through the shell caller.

As per path instructions, “Apply explicit limits to user-controlled … process output.”

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_host_runtime/src/process_port.rs` around lines 248 - 254,
Update execute_local_command and process_output so the request’s output_limit
bounds captured and saved process output on the host path, instead of using only
the fixed preview/save split. Preserve the existing timeout clamp and ensure the
shell caller exercises the per-call limit, including a small limit such as 1
KiB.

Source: Path instructions

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Background log path is reconstructed from the wrong PID ($$ vs $!).

The launch script redirects to bg-$$.log (the launcher sh PID), but the returned pid comes from echo $! (the backgrounded child) and log_path is rebuilt as bg-{pid}.log. $$ != $!, so the reported log path names a file that was never written. The "Started in background: pid X, log Y" message points the model at a dead path.

Emit the resolved path from the script rather than reconstructing it in Rust:

-    let launch_script = format!(
-        "mkdir -p /workspace/.ironclaw && {} >>/workspace/.ironclaw/bg-$$.log 2>&1 & echo $!",
-        wrap_command_for_pgid_isolation(&command),
-    );
+    let launch_script = format!(
+        "mkdir -p /workspace/.ironclaw && {} >>/workspace/.ironclaw/bg-$$.log 2>&1 & \
+         echo \"$! /workspace/.ironclaw/bg-$$.log\"",
+        wrap_command_for_pgid_isolation(&command),
+    );

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_host_runtime/src/sandbox_process/exec_transport.rs` around
lines 299 - 352, Update the background launch flow around launch_script and
BackgroundLaunch so the shell emits both the background child PID ($!) and the
actual log path used by the redirection. Parse the two whitespace-separated
values from pid_output, use the first as pid, and store the second as log_path
instead of reconstructing the path from the PID.

🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Missing "+1" caller-level test for the Sandboxed dispatch arm.

The new dispatch arm at Line 540-542 routes HostedSingleTenantVolumeSandboxed to hosted_single_tenant_volume_sandboxed_build_input, but the only outermost-caller regression test (local_runtime_build_input_with_options_fails_closed_for_volume_profile_when_env_unset, Line 1309-1332) exercises RebornCompositionProfile::HostedSingleTenantVolume only — never the new Sandboxed arm through local_runtime_build_input_with_options. This is exactly the "distinct scenario" the neighboring Volume test was written to protect, and its own module doc frames it as the build-through-the-caller check.

+    #[test]
+    fn local_runtime_build_input_with_options_fails_closed_for_sandboxed_profile_when_env_unset() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let root = dir.path().join("data-root");
+
+        let error = match local_runtime_build_input_with_options(
+            RebornCompositionProfile::HostedSingleTenantVolumeSandboxed,
+            "sandboxed-owner",
+            root,
+            RebornRuntimeProfileOptions::default(),
+        ) {
+            Ok(_) => panic!("the outermost production entry point must also fail closed"),
+            Err(error) => error,
+        };
+
+        assert!(
+            matches!(
+                &error,
+                RebornRuntimeProfileError::MissingSecretMasterKeyEnv { env_name }
+                    if env_name == "IRONCLAW_REBORN_SECRET_MASTER_KEY"
+            ),
+            "expected MissingSecretMasterKeyEnv, got {error:?}"
+        );
+    }

Based on coding guidelines, "Every new feature and bug fix must begin with a test... add a new test only for a genuinely distinct scenario and explain why an existing test could not absorb it" and "Test through the caller when a helper gates a side effect."

Also applies to: 627-671, 1302-1332

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_reborn_composition/src/deployment.rs` around lines 540 - 542,
“Add an outermost-caller regression test covering the new
HostedSingleTenantVolumeSandboxed dispatch arm.” Extend the tests around
local_runtime_build_input_with_options and the existing
local_runtime_build_input_with_options_with_volume_profile test pattern to
invoke RebornCompositionProfile::HostedSingleTenantVolumeSandboxed through
local_runtime_build_input_with_options, asserting the same fail-closed behavior
when the required environment is unset; keep the existing
HostedSingleTenantVolume scenario unchanged.

Source: Coding guidelines

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Soft-enforcement egress proxy: opt-in knob outruns its enforcement point.

SANDBOX_HTTP_PROXY_URL_ENV/SANDBOX_HTTP_PROXY_PORT_ENV only set http_proxy/https_proxy on an otherwise normal bridge-networked container; nothing in this wiring enforces that egress actually goes through the named proxy. Any in-container process that ignores those env vars (raw sockets, curl --noproxy, etc.) gets open egress unless the proxy address given also happens to be backed by a real network-level enforcement point — which the TODO admits doesn't exist yet. Until the allowlist-enforcing proxy lands, enabling either env var trades the safe --network none default for an egress posture that looks allowlisted but isn't actually bounded.

As per coding guidelines: "Fail closed for authentication, approvals, trust, filesystem containment, network policy, secret leases, runtime selection, and adapter identity" and "Do not weaken authentication, origin checks, body limits, rate limits, allowlists, approval leases, secret mediation, or redaction guarantees."

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_reborn_composition/src/sandbox_boot.rs` around lines 25 - 67,
Update the network configuration around SANDBOX_HTTP_PROXY_URL_ENV and
SANDBOX_HTTP_PROXY_PORT_ENV so configuring either variable cannot enable bridge
networking or imply enforced egress before a real allowlist proxy exists.
Preserve the fail-closed RebornRuntimeProcessBinding behavior by retaining the
--network none posture unless network-level proxy enforcement is verified and
explicitly supported. Remove or disable the soft proxy environment wiring in the
TenantSandbox setup and revise nearby documentation to match the safe behavior.

Source: Coding guidelines

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Stale "tenant-level"/"per-tenant" doc wording contradicts the per-user implementation.

The module doc and SANDBOX_MAX_CONCURRENT_ENV's doc comment (lines 20-22) both describe this as a tenant-level/per-tenant ceiling, but apply_sandbox_user_ceiling scopes via ResourceAccount::user(...), and this file's own test explicitly proves a sibling user in the same tenant is unaffected. sandbox_composition.rs correctly documents the same feature as "scoped per-user (not per-tenant)". Update the stale wording here to avoid misleading future readers about the actual quota boundary.

-//! Tenant-level concurrency ceiling for the `hosted-single-tenant-volume-sandboxed`
+//! Per-user concurrency ceiling for the `hosted-single-tenant-volume-sandboxed`
 //! profile (D3-2).

-/// Overrides the sandboxed profile's per-tenant concurrent `SpawnProcess`
+/// Overrides the sandboxed profile's per-user concurrent `SpawnProcess`
 /// ceiling. Unset, or set to a non-positive/unparseable value, falls back to
 /// [`DEFAULT_SANDBOX_MAX_CONCURRENT`].

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_reborn_composition/src/sandbox_quota.rs` around lines 1 - 14,
Update the module documentation and the SANDBOX_MAX_CONCURRENT_ENV documentation
to describe the concurrency ceiling as scoped per user, not tenant-wide or
per-tenant. Keep the implementation in apply_sandbox_user_ceiling unchanged,
including its ResourceAccount::user scoping, and align the wording with
sandbox_composition.rs.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Do not hide persistent-home initialization failures.

A failed cp can leave a partial destination directory; later starts see it and never retry, leaving that user’s Rust environment permanently broken. Remove || true and stage copies atomically before moving them into place.

As per path instructions, the Fail loud invariant requires propagating initialization errors rather than continuing with poisoned state.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docker/process-sandbox-entrypoint.sh` around lines 52 - 54, Update the
persistent-home initialization commands in the entrypoint to propagate mkdir and
copy failures instead of suppressing them. Stage the /home/sandbox/.cargo and
/home/sandbox/.rustup copies in temporary destinations under /workspace/.home,
then atomically move each completed stage into place so failed copies cannot
leave directories that prevent retries.

Source: Path instructions

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make Rust bootstrap fail on download errors.

With /bin/sh, a failed curl can feed an empty script to sh, which exits successfully; the image then builds without Rust. Download first, then execute the installer.

-RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal
+RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs -o /tmp/rustup-init \
+    && sh /tmp/rustup-init -y --profile minimal \
+    && /home/sandbox/.cargo/bin/cargo --version \
+    && rm /tmp/rustup-init

As per path instructions, the Fail loud invariant rejects silent-failure patterns.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Dockerfile.process-sandbox` at line 57, Update the Rust bootstrap RUN command
in Dockerfile.process-sandbox to download the installer with curl into a
temporary file first, then execute that file with sh using the existing rustup
arguments. Preserve strict curl failure behavior and ensure the installer is
only run after a successful download.

Sources: Path instructions, Linters/SAST tools